[RFC] Add BOLT 12 payer proof primitives#4297
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4297 +/- ##
===========================================
+ Coverage 28.02% 86.81% +58.79%
===========================================
Files 126 160 +34
Lines 69960 112368 +42408
Branches 69960 112368 +42408
===========================================
+ Hits 19606 97555 +77949
+ Misses 49020 12249 -36771
- Partials 1334 2564 +1230
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
A few notes, though I didn't dig into the code at a particularly low level.
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
2 times, most recently
from
2324361 to
9f84e19
Compare
Add a Rust CLI tool that generates and verifies test vectors for BOLT 12 payer proofs as specified in lightning/bolts#1295. The tool uses the rust-lightning implementation from lightningdevkit/rust-lightning#4297. Features: - Generate deterministic test vectors with configurable seed - Verify test vectors from JSON files - Support for basic proofs, proofs with notes, and invalid test cases - Uses refund flow for explicit payer key control Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🔔 1st Reminder Hey @valentinewallace! This PR has been waiting for your review. |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Some API comments. I'll review the actual code somewhat later (are we locked on on the spec or is it still in flux at all?), but would be nice to reduce allocations in it first anyway.
|
🔔 2nd Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 9th Reminder Hey @jkczyz! This PR has been waiting for your review. |
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
5 times, most recently
from
fb8c68c to
9ad5c35
Compare
|
🔔 18th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 19th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
| /// | ||
| /// `missing_hashes` must be in DFS (left-to-right recursive traversal) order, | ||
| /// matching the order produced by [`build_tree_with_disclosure`]. | ||
| pub(super) fn reconstruct_merkle_root( |
There was a problem hiding this comment.
please add a fuzzer for reconstruction/deser.
There was a problem hiding this comment.
Done in 207078ba3!
Added a payer_proof_deser target that throws arbitrary bytes at PayerProof::try_from, so we exercise the merkle-root reconstruction from the selective-disclosure data (leaf/missing hashes + omitted markers) and the deser path together.
I wired it the same way as the other BOLT 12 *_deser targets, and I kept it minimal like static_invoice_deser because a PayerProof is a terminal object, so there is no response to build on top of it.
One thing: I did not commit a seed because under hashes_fuzz the hashes are fake, so a real proof do not parse there anyway. But if you think a fake-hashes seed would help the coverage I am happy to add one, what do you think? Thanks!
Add a `payer_proof_deser` fuzz target that runs arbitrary bytes through `PayerProof::try_from`, which reconstructs the invoice merkle root from the selective-disclosure data (leaf/missing hashes and omitted markers), runs the omitted-marker validation, the invoice and payer signature checks, and the TLV deserialization. On a successful parse it asserts the parsed bytes round-trip, matching the other BOLT 12 `*_deser` targets. Wired up the same way as the other BOLT 12 `*_deser` targets: a `do_test` in fuzz/src, a module in fuzz/src/lib.rs, and a `GEN_FAKE_HASHES_TEST` entry regenerating fuzz/targets.h and the fake-hashes bin target. Requested in review on lightningdevkit#4297. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Needs rebase, sadly.
| MerkleError(SelectiveDisclosureError), | ||
| /// The invoice signature is invalid. | ||
| InvalidInvoiceSignature, | ||
| /// Failed to re-derive the payer signing key from the provided nonce and payment ID. | ||
| KeyDerivationFailed, | ||
| /// The given TLV type cannot be included in a payer proof. Carries the offending | ||
| /// type number. Reasons include `PAYER_METADATA_TYPE`, TLVs in `SIGNATURE_TYPES`, | ||
| /// or TLVs in `PAYER_PROOF_DATA_TYPES`. | ||
| DisallowedTlvType(u64), | ||
|
|
||
| /// Error decoding the payer proof. | ||
| DecodeError(DecodeError), |
There was a problem hiding this comment.
nit: in general I'm not a big fan of super granular error enums as a way to communicate developer-relevant data. In general, if an error requires different programatic handling by the caller it should have its own enum variant, if the handling is just the same "just log/print the error and consider it failed" then it doesn't need an enum variant. Same goes double for SelectiveDisclosureError - do we need it at all or would a &'static str suffice?
InvoiceRequest and Refund have payer metadata consisting of an encrypted payment id and, originally, a nonce used to derive the payer signing keys and authenticate any corresponding invoices. The nonce was elided to save space once it was included in the OffersContext of blinded reply paths, but that means verifying a Bolt12Invoice requires state outside the invoice itself. Upcoming payment proofs (lightningdevkit#4297) need the invoice signing keys derivable from the invoice request alone, so include the nonce in the payer metadata again and verify invoices using it rather than the context's nonce. Co-Authored-By: Claude <noreply@anthropic.com>
InvoiceRequest and Refund have payer metadata consisting of an encrypted payment id and, originally, a nonce used to derive the payer signing keys and authenticate any corresponding invoices. The nonce was elided to save space once it was included in the OffersContext of blinded reply paths, but that means verifying a Bolt12Invoice requires state outside the invoice itself. Upcoming payment proofs (lightningdevkit#4297) need the invoice signing keys derivable from the invoice request alone, so include the nonce in the payer metadata again and verify invoices using it rather than the context's nonce. This breaks verification of invoices for invoice requests and refunds with blinded paths created by prior versions, as their payer metadata lacks the nonce; such payments will fail and must be retried with a new payment id. Refunds without blinded paths are unaffected, as their metadata always included the nonce. Co-Authored-By: Claude <noreply@anthropic.com>
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
2 times, most recently
from
93f4104 to
73eea68
Compare
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
2 times, most recently
from
8d543ce to
5aedb03
Compare
| // Build TLV stream matching spec example structure | ||
| // TLVs: 0, 10, 20, 30, 40, 50, 60 | ||
| let mut tlv_bytes = Vec::new(); | ||
| tlv_bytes.extend_from_slice(&[0x00, 0x04, 0x00, 0x00, 0x00, 0x00]); // TLV 0 | ||
| tlv_bytes.extend_from_slice(&[0x0a, 0x02, 0x00, 0x00]); // TLV 10 | ||
| tlv_bytes.extend_from_slice(&[0x14, 0x02, 0x00, 0x00]); // TLV 20 | ||
| tlv_bytes.extend_from_slice(&[0x1e, 0x02, 0x00, 0x00]); // TLV 30 | ||
| tlv_bytes.extend_from_slice(&[0x28, 0x02, 0x00, 0x00]); // TLV 40 | ||
| tlv_bytes.extend_from_slice(&[0x32, 0x02, 0x00, 0x00]); // TLV 50 | ||
| tlv_bytes.extend_from_slice(&[0x3c, 0x02, 0x00, 0x00]); // TLV 60 |
There was a problem hiding this comment.
Could you also test something that exercises all the omitted marker edge cases?
There was a problem hiding this comment.
Done. Added edge-case coverage for leading runs, empty markers, gap jumps, zero, duplicate/non-ascending markers, included-type collisions, and non-minimized markers.
There was a problem hiding this comment.
Could you make sure those tests roundtrip, too?
Move the invoice/refund payer key derivation logic into reusable helpers so payer proofs can derive the same signing keys without duplicating the metadata and signer flow.
| let mut tlv_stream = tlv_stream.peekable(); | ||
| let nonce_tag = tagged_hash_engine(sha256::Hash::from_engine({ | ||
| let first_tlv_record = tlv_stream.peek().unwrap(); | ||
| let mut engine = sha256::Hash::engine(); | ||
| engine.input("LnNonce".as_bytes()); | ||
| engine.input(first_tlv_record.record_bytes); | ||
| engine | ||
| })); | ||
| let leaf_tag = tagged_hash_engine(sha256::Hash::hash("LnLeaf".as_bytes())); | ||
| let branch_tag = tagged_hash_engine(sha256::Hash::hash("LnBranch".as_bytes())); | ||
|
|
||
| let mut leaves = Vec::new(); | ||
| for record in tlv_stream.filter(|record| !SIGNATURE_TYPES.contains(&record.r#type)) { | ||
| leaves.push(tagged_hash_from_engine(leaf_tag.clone(), &record.record_bytes)); | ||
| leaves.push(tagged_hash_from_engine(nonce_tag.clone(), &record.type_bytes)); | ||
| } | ||
| let (tlv_data, branch_tag) = merkle_tlv_data(tlv_stream, |_| false); | ||
| let mut leaves: Vec<sha256::Hash> = tlv_data.iter().map(|data| data.per_tlv_hash).collect(); |
There was a problem hiding this comment.
Hmm... not sure if we should take the refactor this far if it results in allocating two Vecs -- here and Vec::with_capacity in merkle_tlv_data.
There was a problem hiding this comment.
OK, let me know how this looks now 7d2749c
Extend the BOLT 12 merkle module with selective-disclosure support: build the full merkle tree from a TLV stream, compute the omitted-TLV markers and the minimal set of missing hashes for omitted subtrees, and reconstruct the merkle root from a partial disclosure. These are the primitives a payer proof is built on. Co-Authored-By: Rusty Russell <rusty@rustcorp.com.au> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: OpenAI Codex <codex@openai.com>
Add the `payer_proof` module: `PayerProof`/`UnsignedPayerProof`, the `PayerProofBuilder` (with selective disclosure and a derived-key path), bech32 `lnp` encoding, and parse-time verification, implementing the payer proof extension to BOLT 12 (lightning/bolts#1295). Also exposes the offer/invoice TLV-type constants and an invoice-bytes accessor used to build proofs, and a `Sha256` `Writeable`/`Readable` impl for the proof hashes. Co-Authored-By: Rusty Russell <rusty@rustcorp.com.au> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: OpenAI Codex <codex@openai.com>
Carry the paid `Bolt12Invoice` through the outbound payment so it survives restarts, and surface it as a `PaidBolt12Invoice` on `Event::PaymentSent` so the payer can build a payer proof. The payer signing key is re-derived from the invoice's own payer metadata, so no extra key material is stored. `PaidBolt12Invoice` now lives in `offers::payer_proof`; existing async payment tests and a test helper are updated to construct it via the new API. Adds an end-to-end test that pays a BOLT 12 offer and builds + verifies a payer proof from the resulting `Event::PaymentSent`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Throw arbitrary bytes at `PayerProof::try_from` to exercise the merkle-root reconstruction and the deserialization path together. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: OpenAI Codex <codex@openai.com>
| while inc_idx < included_types.len() || mrk_idx < omitted_markers.len() { | ||
| if mrk_idx >= omitted_markers.len() { | ||
| // No more markers, remaining positions are included | ||
| positions.push(true); | ||
| inc_idx += 1; | ||
| } else if inc_idx >= included_types.len() { | ||
| // No more included types, remaining positions are omitted | ||
| positions.push(false); | ||
| prev_marker = omitted_markers[mrk_idx]; | ||
| mrk_idx += 1; | ||
| } else { | ||
| let marker = omitted_markers[mrk_idx]; | ||
| let inc_type = included_types[inc_idx]; | ||
|
|
||
| if marker == next_marker(prev_marker) { | ||
| // Continuation of current run → this position is omitted | ||
| positions.push(false); | ||
| prev_marker = marker; | ||
| mrk_idx += 1; | ||
| } else { | ||
| // Jump detected! An included TLV comes before this marker. | ||
| // After the included type, prev_marker resets to that type, | ||
| // so the marker will be processed as a continuation next iteration. | ||
| positions.push(true); | ||
| prev_marker = inc_type; | ||
| inc_idx += 1; | ||
| // Don't advance mrk_idx - same marker will be continuation next | ||
| } | ||
| } |
There was a problem hiding this comment.
Can't this drift from what's essentially the same loop in reconstruct_merkle_root? We should DRY this up.
| // Test the type validation logic directly. | ||
| fn check_include_type(tlv_type: u64) -> Result<(), PayerProofError> { | ||
| if tlv_type == PAYER_METADATA_TYPE | ||
| || SIGNATURE_TYPES.contains(&tlv_type) | ||
| || PAYER_PROOF_DATA_TYPES.contains(&tlv_type) | ||
| { | ||
| return Err(PayerProofError::DisallowedTlvType(tlv_type)); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Shouldn't we be testing production code?
| // Build TLV stream matching spec example structure | ||
| // TLVs: 0, 10, 20, 30, 40, 50, 60 | ||
| let mut tlv_bytes = Vec::new(); | ||
| tlv_bytes.extend_from_slice(&[0x00, 0x04, 0x00, 0x00, 0x00, 0x00]); // TLV 0 | ||
| tlv_bytes.extend_from_slice(&[0x0a, 0x02, 0x00, 0x00]); // TLV 10 | ||
| tlv_bytes.extend_from_slice(&[0x14, 0x02, 0x00, 0x00]); // TLV 20 | ||
| tlv_bytes.extend_from_slice(&[0x1e, 0x02, 0x00, 0x00]); // TLV 30 | ||
| tlv_bytes.extend_from_slice(&[0x28, 0x02, 0x00, 0x00]); // TLV 40 | ||
| tlv_bytes.extend_from_slice(&[0x32, 0x02, 0x00, 0x00]); // TLV 50 | ||
| tlv_bytes.extend_from_slice(&[0x3c, 0x02, 0x00, 0x00]); // TLV 60 |
There was a problem hiding this comment.
Could you make sure those tests roundtrip, too?
| #[inline] | ||
| pub fn do_test<Out: test_logger::Output>(data: &[u8], _out: Out) { | ||
| if let Ok(payer_proof) = PayerProof::try_from(data.to_vec()) { | ||
| assert_eq!(data, payer_proof.bytes()); |
There was a problem hiding this comment.
Aren't these trivially equal? We should do what the invoice fuzzer does:
Though in practice those should be the exact same bytes. This may require a Writeable implementation like our other BOLT12 types.
|
|
||
| #[inline] | ||
| pub fn do_test<Out: test_logger::Output>(data: &[u8], _out: Out) { | ||
| if let Ok(payer_proof) = PayerProof::try_from(data.to_vec()) { |
There was a problem hiding this comment.
We should also update the invoice fuzzer to build a payer proof, just like how the offer fuzzer builds an invoice request and how the invoice request fuzzer builds an invoice.
| } | ||
|
|
||
| /// Tests the full payer proof lifecycle: offer -> invoice_request -> invoice -> payment -> | ||
| /// proof creation with derived key signing -> verification -> bech32 round-trip. |
There was a problem hiding this comment.
Doesn't seem like the test actually does the round trip, so aren't testing verification.
6 participants
This is a first draft implementation of the payer proof extension to BOLT 12 as proposed in lightning/bolts#1295. The goal is to get early feedback on the API design before the spec is finalized.
Payer proofs allow proving that a BOLT 12 invoice was paid by demonstrating possession of:
This PR adds the core building blocks:
This is explicitly a PoC to validate the API surface - the spec itself is still being refined. Looking for feedback on:
cc @TheBlueMatt @jkczyz